Is this a problem only on on macOS 14? That's out of date now. Is it a problem on 15.7.x and 26.x?

Test Results

  182 files  +   31    182 suites  +31   26m 8s ⏱️ + 6m 13s
4 729 tests ±    0  4 706 ✅ ±    0   23 💤 ± 0  0 ❌ ±0 
6 848 runs  +1 125  6 685 ✅ +1 084  163 💤 +41  0 ❌ ±0 

Results for commit 4ff9ad6. ± Comparison against base commit 9ab3898.

♻️ This comment has been updated with latest results.

No, this problem is reproducible with MacOS 26.5, too. I have not tried on other systems though.

Are these failing Windows tests caused by this PR?

Is this worth merging?

This change seems to be modying core key handling of StyledText. Without any explanation of the conceptual idea of the fix, why it may be sound, and how the issue-to-be-solved can be reproduced, I am not sure if anyone will consider this ready to be merged.

Please find steps to reproduce in #3306. I thought to have explained the reason in the commit message.

Thank you for the reference. I usually do not look at the commit message but only at the PR description, and I did not recognize that there seems to be an implicit reference to the issue in the PR title. It helps to just have a proper reference in the description.

Still I do not see an explanation of the conceptual idea of the fix and why it may be sound in the issue, the commit message or the PR description. It just says that some combination of keycode and character does not work, but that does not say anything about the concept of the fix.

Before my fix the event had the keycode 12385, so it took the if statement. But for that keycode, no keybinding existed, so nothing was done. Now it first tries with the keycode as before, but if no keybinding was found, it tries to look for a character keybinding, too. And then it finds something.

Yes, so far I understand how exactly the problematic case works because of the change.
My question is regarding how the concept looks like for how the method is supposed to operate in general now. As said, this looks like core interaction functionality of the StyledText to me, so we have to understand how that method is supposed to operate now, such that all existing used cases keep working. Since you seem to have already invested the time to understand how the control and data flow of this method works in detail to be able to provide the fix proposal without breaking existing functionality, it would be great to just write that down and share it, so that a potential reviewer that does not have to build this whole knowledge again.

Sorry, I don't understand what further information you need. If this is not sufficient for you, just close this PR. I had the hope that other users, especially in Japan, also would like to benefit from my fix.

I tested this on Mac using the Japanese Kana keyboard layout. Without this PR pressing MOD1+A doesn't perform the Select All action, with this PR it does.

I would think that MOD1 + A should perform the Select All action in all keyboard layouts.

@HeikoKlare it seems like this fixes an edge case with the Japanese Kana keyboard layout (and possibly others) in the stand-alone use of the StyledText control. As long as it doesn't cause a regression I would have thought this would be useful in that case?

Further investigations...

A workaround:

styledText.setKeyBinding(12385 | SWT.MOD1, ST.SELECT_ALL);

Seeing as how some folks are using AI to summarise issues/PRs, I'll add this one...

What happens

When you press Cmd + A (for "Select All") on a Japanese Kana keyboard layout:

• event.keyCode = 12385 (a Unicode value corresponding to a Japanese character, like ち / "chi")
• event.character = 'a' (the expected Latin character)

The original code in StyledText.handleKey(Event event) only checked key bindings using event.keyCode | event.stateMask first. Since there's no key binding registered for 12385 | SWT.MOD1, the shortcut does nothing.

The fix in this PR

The PR changes handleKey to fall back to checking the character if no binding is found for the keyCode.

This makes standard shortcuts (like Cmd+A, Cmd+C, etc.) work even when the underlying macOS event reports a Japanese keycode for Latin keys under certain input methods.

Why this occurs on Japanese layouts

macOS (and other platforms with complex input methods / IME) often sends the "committed" or native character code in keyCode when using kana/kanji input modes, while still providing the base Latin character for compatibility.

Eclipse's key binding system wasn't handling this mismatch for StyledText.
This is a common class of issue with CJK (Chinese/Japanese/Korean) input methods in cross-platform toolkits.

it seems like this fixes an edge case with the Japanese Kana keyboard layout (and possibly others) in the stand-alone use of the StyledText control. As long as it doesn't cause a regression I would have thought this would be useful in that case?

Sure, it's definitely useful. But you mention the important point as well "as long as doesn't cause a regression". The code does not just add a specific if statement for the edge or the like, it changes how the method processes key event parameters in general. And I am not able to spend the time to get all the knowledge of how this works (including control and data flow of the method), in particular because the creator of such a change needs to have done that anyone and would the person I would expect to post the information together with the change proposal.

So if there is someone who is confident with the change or invests the time into fully understanding the impact, it's of course fine for me to merge this. I just wanted to point out that I am not sure if there will be someone doing that (and that I least I will not be that person).

Maybe someone can simply merge it and we quickly will notice if it will cause any regressions we are not aware of now. We are using this PR for SmartGit 26.2 internally and did not notice any regression yet. I'm sure, without trying we won't find out.

This change fixes a specific case that no one seems to have noticed/reported for years. It adapts hows combinations of keyCode and character are processed, without any explanation about the existing and new data flows and processings of those combinations. Merging this with just some testing and with involving any conceptual understanding of the change means that we proceed in a try-and-error manner (without any need, as one could just read and understand the method and derive the necessary knowledge). Not unlikely that this change will replace the bug for one long-unnoticed case with another one that will again be noticed after quite a while as the case occurs that seldom.
As an example, we just have a case where we did a change three months ago (also in key event processing) and only now with the release we see a report for a regression that this introduced:

• 'Paste' is executed twice eclipse.platform.ui#4080

With my previous comments, I always assumed that the conceptual knowledge about soundness of this change exists and is just not shared yet, but it more and more sounds as if the fix was just done in a try-and-error manner rather than a conceptual understanding of the affected method's behavior.

So as the last comment from my side on this: I strongly discourage doing such a try-and-error change without having anyone understanding the change conceptually. But I also won't block this, so if anyone invests the time to get that understanding, already has it, or feels confident in any other way, feel free to merge.